Release TVar locks for all exceptions - #1116
Conversation
bensheldon
left a comment
bensheldon
left a comment
There was a problem hiding this comment.
This looks ok.
It should be raise not raise e on the next line, right?
Are there other instances of this pattern we should update? Not necessarily jammed into this PR, but while it's top of mind, the help would be appreciated.
|
Agreed. I changed raise e to a bare raise and removed the now-unused rescue variable so the original backtrace is preserved. I also added a regression that writes a TVar, raises Interrupt, verifies the write is rolled back, and then performs a second transaction to prove the lock was released. The focused suite passes: 15 examples, 0 failures. I audited the other Exception rescues in lib: the worker and promise sites already deliberately distinguish non-StandardError exceptions and do their cleanup; I did not find another transactional/resource-release instance of this exact StandardError-only pattern. |
2 participants
Aborts the transaction before re-raising exceptions outside StandardError as well, preventing TVar locks from remaining held after Interrupt, SystemExit, or other Exception subclasses.
Verified by focused TVar specs and models; the review composite passes 2,796 examples with no failures.